feat(orchestrator): Surface waiting background work#4378
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
isBackgroundCapableTurnItemType causes replayable waiting runs to have their background-capable turn items (command_execution, dynamic_tool, subagent) cancelled, even though those runs were intentionally skipped from terminalization because a pending/running checkpoint effect still exists. When a waiting run has a replayable checkpoint, reconcileProjection omits it from runs, so its ID is absent from recoveredNonterminalRunIds. The later stale-item loop then matches that run's items via isBackgroundCapableTurnItemType + isNonterminalTurnItemStatus and emits turn-item.updated cancellation events, destroying items that belong to a still-recoverable run and checkpoint. The stale-item loop needs to also exclude turn items whose runId belongs to a replayable waiting run, not just those already in recoveredNonterminalRunIds.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderRuntimeRecoveryService.ts around line 304:
`isBackgroundCapableTurnItemType` causes replayable `waiting` runs to have their background-capable turn items (command_execution, dynamic_tool, subagent) cancelled, even though those runs were intentionally skipped from terminalization because a pending/running checkpoint effect still exists. When a `waiting` run has a replayable checkpoint, `reconcileProjection` omits it from `runs`, so its ID is absent from `recoveredNonterminalRunIds`. The later stale-item loop then matches that run's items via `isBackgroundCapableTurnItemType` + `isNonterminalTurnItemStatus` and emits `turn-item.updated` cancellation events, destroying items that belong to a still-recoverable run and checkpoint. The stale-item loop needs to also exclude turn items whose `runId` belongs to a replayable `waiting` run, not just those already in `recoveredNonterminalRunIds`.
There was a problem hiding this comment.
Thanks for the careful trace. The control flow you describe is accurate, but I don't think the cancellation is a defect, so I'm leaving this as-is.
A replayable checkpoint.capture cannot revive the provider process or the background work it owned. Its only run-level action is finalizing the already-settled root run: CheckpointCaptureService emits run.updated with status: "completed". So on restart, the command_execution / dynamic_tool / subagent items belonging to that run are orphaned regardless, because the process that could complete them is gone.
That makes cancelling them the correct cleanup rather than the destruction of recoverable work. The skip set exists to avoid emitting duplicate cancellation events for runs already terminalized in the loop above, not to exempt preserved runs from item cleanup. Exempting them as suggested would leave permanently nonterminal cards for work that can never finish, which is a worse outcome than the current behavior: a stuck spinner with no path to resolution.
The sequence on restart during that window is: background items are cancelled, the roster clears, and the durable checkpoint effect replays and completes the root run. That matches "the server died while background work was in flight," which is what happened.
I did act on your neighbouring comment about the same loop (the missing subagent.updated for settled runs); that one is a real gap and is being fixed.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
e46af0a to
f318877
Compare
1e58e65 to
a286c60
Compare
f318877 to
2bf4cdf
Compare
4f72b01 to
dcb197d
Compare
Replays the amended PR pingdotgg#4378 wake-settlement fix onto the v2.1 trial. A terminal task notification no longer opens an opaque-task continuation unless native user, assistant, or result output proves Claude began the wake turn.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
bd45aae to
29cfba8
Compare
29cfba8 to
123245f
Compare
123245f to
fc4cc2c
Compare
fc4cc2c to
2f07ff6
Compare
2f07ff6 to
a7d4eb3
Compare
a7d4eb3 to
c8e3bb2
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c8e3bb2. Configure here.
| ? resetBackgroundTaskStateForNativeThreadProcess(nativeThreadId, { | ||
| status: "idle", | ||
| }) | ||
| : Effect.void, |
There was a problem hiding this comment.
Wake state survives failed replace
Medium Severity
Same-native-thread openQuery replacement closes the live CLI, then on open failure only calls resetBackgroundTaskStateForNativeThreadProcess. Unlike sibling replacement, it never calls clearWakeStateForNativeThread, so wakeBuffers and requestedContinuations can survive a dead process. Session-wide hasPendingBackgroundWork still pins on buffered assistant/user/result frames, so idle release can stay stuck until the pin timeout after a failed model or policy switch that races an already-buffered wake.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit c8e3bb2. Configure here.
1 participant
Summary
background work remains.
and Claude subagents.
Bash.
web sidebar while preserving Working for an active turn.
Relationship to #4415
This is the orchestrator-v2 counterpart to Theo's
#4415. That PR owns the current
SidebarV2 and mobile-v2 Waiting presentation. This PR carries the equivalent
provider-thread and projection contract for orchestrator v2.
The active desktop build still renders the classic
Sidebar.tsxlist, whichdoes not consume #4415's SidebarV2 resolver. This PR adds only the shared
classic status-pill bridge required to display the already-normalized v2 roster.
It does not modify SidebarV2 or the native mobile list.
Problem and Fix
pendingBackgroundTasks, after active Working and Connecting precedence.pendingBackgroundTaskswith an empty-array default for backward compatibility.Defensive Fixes
Manual test guide
Browsable scenario guide: https://nam7nt0rbtm6.postplan.dev/
This PR owns the Waiting state, so most background scenarios in the guide assert
it. The ones that matter, across all three providers:
Waiting must appear
5, Background subagent post-settle,
6, Resume completed subagent,
7, App-owned delegated child.
4,
5,
6,
7. Scenarios 5 and 6
are the two-task cases, where Waiting must report both.
and 5, Steer while run is held open.
Waiting must NOT appear. These are the false-positive guards, and they are
the easier half to regress:
3,
4: work that
finishes before the root settles.
same, for a background command.
and 3: persistent Grok
monitors are intentionally excluded from the roster.
Known failure, not caused by this PR
After a mid-turn steer the row can go blank, showing neither Working nor
Waiting. That is the status resolver correctly reporting a terminal run and an
empty roster; the run settled early upstream of it. Tracked separately, and
deliberately not patched here, because showing Working without a live run
would mislabel genuinely idle threads.
Validation
suites pass. The final Claude adapter suite passes 53 tests, and the
classic-sidebar suite passes 87 tests.
typecheck passes, and
git diff --checkpasses.subagents, Codex background exec and subagents, and Grok continuations and
subagents.
the classic sidebar row displayed static, muted Waiting after root settlement,
then returned to its timestamp after the roster cleared.
that ended with notification-only traffic. Waiting cleared after run 2, no
empty run 3 appeared, and the provider stayed idle through a 35-second
residual window.
notification-only path and its regression guards. Two independent scenarios
drove a real nonpersistent Monitor whose terminal notification arrived with no
native wake frames: both cleared Waiting without opening a run, and each held
at two completed runs, idle with an empty roster, through a residual window of
about 176 seconds. The prior defect left an empty run active for 225 seconds.
once across a separate follow-up turn, a resumed subagent still wakes, and Stop
during a blocking TaskOutput still leaves two distinct clean recovery turns.
Every wake log marker in the round was attributable to an expected wake, with
none unattributed.
server: the row rendered a static muted Waiting pill with dot class
bg-sidebar-muted-foregroundand no pulse while the roster was nonempty, andtitle plus timestamp only after it cleared. An adjacent thread rendered Working
in the same view, confirming the states are visually distinct.
RunExecutionServiceoverlap that will need conflict resolution if thebranches are combined.
The failing Check job is the known Vite+ stdout panic ("This is a bug in Vite+,
not your code") raised while printing pre-existing warnings, not a lint or type
error from this branch.
vp checkon this exact tree reports 0 errors and 70pre-existing warnings, and Test, Release Smoke, Mobile Native Static Analysis,
and both Macroscope checks pass on this head.
This branch is based on the live
t3code/codex-turn-mappingtip. It does notsemantically depend on #4218.
Note
High Risk
Changes orchestration adapter control flow for post-settle wake, carryover projection, and idle-release pinning; regressions could mis-project subagents, drop wake buffers, or wedge sessions.
Overview
Extends ACP v2 post-settle continuation so carryover subagents stay accurate after an interrupted or settled root turn: carryover now records root terminal status, tracks
terminalStatusProjected, and splits in-memory updates from event projection so buffered wake traffic does not double-emit turn items.bufferPostSettleWakereturns a structured outcome (buffer / offer continuation / stop processing); post-settle session updates sync carryover before offering continuations, andhasPendingBackgroundWorkstays true while live carryover or unprojected terminals exist. Every new turn drains the session-scoped wake buffer and projects deferred carryover terminals on attach (user or provider continuation).Claude adapter tests add broad coverage for
pendingBackgroundTaskson the provider thread (SDK roster snapshots,task_startedfallback, fail/interrupt clears, sibling process replacement), wake gating (continuation only after native assistant output, not notification-only opaque work), process/query replacement and failed-open roster resets, and mixedlocal_bashvs subagent classification across buffered notifications.Reviewed by Cursor Bugbot for commit c8e3bb2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Surface waiting background work in the orchestrator, sidebar, and chat timeline
OrchestrationV2PendingBackgroundTaskto the contracts layer and propagates apendingBackgroundTasksroster through provider threads and thread shells, with backward-compatible empty-array defaults.derivePendingBackgroundWork(inorchestrationV2PendingBackgroundWork.ts) to compute a deduplicated list of pending tasks from provider thread rosters and nonterminal turn items, gated on post-settlement run status.ClaudeAdapterV2from a single ID set to per-native-thread roster maps with wake eligibility and replay tombstones; emitsprovider_thread.updatedwith the current roster and correct active/idle status at every turn boundary.AcpAdapterV2to trackterminalStatusProjectedon carryover subagents, enabling deferred projection and correcthasPendingBackgroundWorkpinning until terminal statuses are surfaced.writeIfProviderThreadOwnertoEventSinkV2for ownership-gated provider thread writes, and uses it inRunExecutionServiceto keep ingestion open only while the provider's thread-scoped pending-work probe reports work remaining.ProviderRuntimeRecoveryService) now cancels stale background-capable turn items on settled runs and clearspendingBackgroundTaskson all provider threads.getShellSnapshotandderiveThreadRuntimecomputependingBackgroundTasksper thread;shellRuntimereturnsidleinstead of the cached status when a post-settlement roster is nonempty.waiting-backgroundrow rendered as a pulsating indicator; sidebar gains aWaitingstatus pill ranked betweenWorkingandPlan Ready.hasPendingBackgroundWorkForThreadprobe returning false; if the probe sticks, streams remain open indefinitely.Macroscope summarized c8e3bb2.